Default to padding blinded paths #4213

TheBlueMatt · 2025-11-10T15:23:43Z

After much discussion in #3246 we mostly decided to allow
downstream developers to override whatever decisions the
`DefaultMessageRouter` makes regarding blinded path selection by
providing easy overrides for the selected `OnionMessageRouter`. We
did not, however, actually select good defaults for
`DefaultMessageRouter`.

Here we add those defaults, taking advantage of the
`MessageContext` we're given to detect why we're building a blinded
path and selecting blinding and compaction parameters based on it.

Specifically, if the blinded path is not being built for an offers
context, we always use a non-compact blinded path and always pad it
to four hops (including the recipient).

However, if the blinded path is being built for an `Offers` context
which implies it might need to fit in a QR code (or, worse, a
payment onion), we reduce our padding and try to build a compact
blinded path if possible.

We retain the `NodeIdMessageRouter` to disable compact blinded path
creation but use the same path-padding heuristic as for
`DefaultMessageRouter`.

Because they end up both being used to validate a `Bolt12Invoice`, we ended up with a single `OffersContext` both for inclusion in a `Refund` and an `InvoiceRequest`. However, this is ambiguous, and while it doesn't seem like an issue, it also seems like a nice property to only use a given `OffersContext` in one place. Further, in the next commit, we use `OffersContext` to figure out what we're building a blinded path for and changing behavior based on it, so its nice to be unambiguous. Thus, we split the single existing context into `OutboundPaymentInRefund` and `OutboundPaymentInInvReq`.

ldk-reviews-bot · 2025-11-10T15:23:46Z

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignment, please click here.

codecov · 2025-11-10T18:07:04Z

Codecov Report

❌ Patch coverage is 96.58537% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.35%. Comparing base (e42e74e) to head (f7620fe).
⚠️ Report is 145 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/offers_tests.rs	94.11%	2 Missing ⚠️
lightning/src/offers/invoice.rs	87.50%	2 Missing ⚠️
lightning-dns-resolver/src/lib.rs	0.00%	1 Missing ⚠️
lightning/src/blinded_path/message.rs	97.50%	1 Missing ⚠️
lightning/src/offers/flow.rs	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4213      +/-   ##
==========================================
+ Coverage   89.33%   89.35%   +0.02%     
==========================================
  Files         180      180              
  Lines      138055   139861    +1806     
  Branches   138055   139861    +1806     
==========================================
+ Hits       123326   124972    +1646     
- Misses      12122    12298     +176     
+ Partials     2607     2591      -16

Flag	Coverage Δ
fuzzing	`35.17% <2.29%> (+1.60%)`	⬆️
tests	`88.69% <96.58%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ldk-reviews-bot · 2025-11-12T15:35:30Z

🔔 1st Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-15T00:00:27Z

🔔 2nd Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-17T00:01:28Z

🔔 3rd Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-19T00:02:38Z

🔔 4th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-22T00:00:16Z

🔔 5th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-24T00:01:03Z

🔔 6th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-26T00:01:28Z

🔔 7th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-29T00:00:54Z

🔔 8th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-12-01T00:01:40Z

🔔 9th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-12-03T00:02:10Z

🔔 10th Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

lightning/src/onion_message/messenger.rs

valentinewallace · 2025-12-03T01:28:49Z

lightning/src/ln/offers_tests.rs

+	expected_path_length: usize,
+) -> bool {
+	let introduction_node_id = resolve_introduction_node(lookup_node, path);
+	introduction_node_id == expected_introduction_node


We previously had a check that the intro node was encoded as an scid, should we restore that? Or maybe it changed due to the never-compact option

We quite often use non-compact paths now, as we generally try to use them any time where we're not space-constrained, so such a check fails a handful of test.

lightning/src/blinded_path/message.rs

valentinewallace · 2025-12-03T01:47:01Z

lightning/src/onion_message/messenger.rs

-/// message, and thus an `Err` is returned.
+/// message, and thus an `Err` is returned. The impact of this may be somewhat muted when
+/// additional padding is added to the blinded path, but this protection is not complete.
 pub struct NodeIdMessageRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref>


Not related to the PR per se, I'm wondering if we can get rid of this struct? I don't see it used anywhere and the docs don't indicate when it should be used

The intent is that someone may want to override the router (eg using the flow's create_offer_builder_using_router).

Ok, I still don't think the NodeIdMessageRouter docs make it clear when a user would want to actually do that. I assume it's for more stable blinded paths, i.e. avoiding using scids in a blinded message path ever.

Oh, sorry I thought you meant how would it be used, not where.

ldk-reviews-bot · 2025-12-03T01:47:13Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

ldk-reviews-bot · 2025-12-08T22:31:09Z

🔔 1st Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

valentinewallace

LGTM after squash. May want someone more familiar with the previous padding/dummy hop work to take a look

lightning/src/onion_message/functional_tests.rs

After much discussion in lightningdevkit#3246 we mostly decided to allow downstream developers to override whatever decisions the `DefaultMessageRouter` makes regarding blinded path selection by providing easy overrides for the selected `OnionMessageRouter`. We did not, however, actually select good defaults for `DefaultMessageRouter`. Here we add those defaults, taking advantage of the `MessageContext` we're given to detect why we're building a blinded path and selecting blinding and compaction parameters based on it. Specifically, if the blinded path is not being built for an offers context, we always use a non-compact blinded path and always pad it to four hops (including the recipient). However, if the blinded path is being built for an `Offers` context which implies it might need to fit in a QR code (or, worse, a payment onion), we reduce our padding and try to build a compact blinded path if possible. We retain the `NodeIdMessageRouter` to disable compact blinded path creation but use the same path-padding heuristic as for `DefaultMessageRouter`.

TheBlueMatt · 2025-12-09T21:19:18Z

Squashed and updated a few docs/comments:

$ git diff-tree -U2 649ba3478 8efb3e171
diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs
index ffdb2f6403..75e2aaf3c5 100644
--- a/lightning/src/onion_message/functional_tests.rs
+++ b/lightning/src/onion_message/functional_tests.rs
@@ -696,5 +696,6 @@ fn test_blinded_path_padding_for_full_length_path() {
 #[test]
 fn test_blinded_path_compact_padding() {
-	// Check that for a blinded path with compact padding, no extra padding is applied.
+	// Check that for a blinded path with non-SCID intermediate hops with compact padding, no extra
+	// padding is applied.
 	let nodes = create_nodes(4);

@@ -729,5 +730,6 @@ fn test_blinded_path_compact_padding() {
 #[test]
 fn test_compact_blinded_path_compact_padding() {
-	// Check that for a blinded path with compact padding, no extra padding is applied.
+	// Check that for a blinded path with SCID intermediate hops with compact padding, no extra
+	// padding is applied.
 	let nodes = create_nodes(4);

diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs
index d6d4314196..03fc8dd4f7 100644
--- a/lightning/src/onion_message/messenger.rs
+++ b/lightning/src/onion_message/messenger.rs
@@ -773,4 +773,7 @@ where
 /// paths.
 ///
+/// This may be useful in cases where you want a long-lived blinded path and anticipate channel(s)
+/// may close, but connections to specific peers will remain stable.
+///
 /// This message router can only route to a directly connected [`Destination`].
 ///

ldk-reviews-bot · 2025-12-09T21:20:32Z

✅ Added second reviewer: @wpaulino

jkczyz · 2025-12-11T15:44:23Z

lightning/src/offers/flow.rs

+			Some(&OffersContext::OutboundPaymentInRefund { payment_id, nonce, .. }) => {
+				if invoice.is_for_refund() {
+					invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx)
+				} else {
+					Err(())
+				}
 			},


Will need to mention in the release notes that this breaks existing refunds.

The TLV type matches for refunds, so it breaks issued-but-unresponded-to invoice requests.

jkczyz · 2025-12-11T15:47:37Z

lightning/src/blinded_path/message.rs

+	OutboundPaymentInRefund {
+		/// Payment ID used when creating a [`Refund`].
 		///
 		/// [`Refund`]: crate::offers::refund::Refund
-		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		payment_id: PaymentId,

-		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] or
-		/// [`InvoiceRequest`] and for deriving their signing keys.
+		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] and
+		/// for deriving its signing keys.
 		///
 		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 		/// [`Refund`]: crate::offers::refund::Refund
+		nonce: Nonce,
+	},
+	/// Context used by a [`BlindedMessagePath`] as a reply path for an [`InvoiceRequest`].
+	///
+	/// This variant is intended to be received when handling a [`Bolt12Invoice`] or an
+	/// [`InvoiceError`].
+	///
+	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
+	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
+	/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
+	OutboundPaymentInInvReq {


How about OutboundPaymentForOffer and OutboundPaymentForRefund?

Hmm, guess I don't feel strongly. Neither should be ambiguous - we don't use invreqs for refunds, I kept it equivalent cause both inv-reqs and refunds are the same "step" in the bolt 12 flow. If you feel like its more consistent with the rest of our code I'm happy to change it.

jkczyz · 2025-12-11T15:55:28Z

lightning/src/ln/offers_tests.rs

 		&& matches!(path.introduction_node(), IntroductionNode::DirectedShortChannelId(..))
 }

+fn check_dummy_hopd_path_length<'a, 'b, 'c>(


~~s/hopd/hops~~

edit: Thought this was a typo. Or is it? If so, would read better as "hopped".

Makes it sound like the dummy is hopping over the path, but sure 😂

lightning/src/onion_message/messenger.rs

lightning/src/blinded_path/message.rs

jkczyz · 2025-12-11T16:26:34Z

lightning/src/blinded_path/message.rs

+		// We can only pad by a minimum of two bytes (we can only go from no-TLV to a type + length
+		// byte). Thus, if there are any intermediate hops that need to be padded by exactly one


This could use a better explanation. Not sure if I quite understand the parenthetical. Is this referring to TLVs inside ForwardTlvs? Are you saying an intermediary node can see how much padding was added and infer something about the next hop?

Hmm, do you have any suggested better phrasing? The point is we can never pad by a single byte, so if we need to pad by a single byte, we have to instead just pad everything by 1/2.

lightning/src/blinded_path/message.rs

If we're building a blinded message path with extra dummy hops, we have to ensure we at least hide the length of the data in pre-final hops as otherwise the dummy hops are trivially obvious. Here we do so, taking an extra `bool` parameter to `BlindedMessagePath` constructors to decide whether to pad every hop to the existing `MESSAGE_PADDING_ROUND_OFF` or whether to only ensure that each non-final hop has an identical hop data length. In cases where the `DefaultMessageRouter` opts to use compact paths, it now also selects compact padding, whether short channel IDs are available or not.

ldk-reviews-bot requested a review from valentinewallace November 10, 2025 15:34

TheBlueMatt force-pushed the 2025-10-pad-use-non-compact-bps branch from 65206f0 to c7d1621 Compare November 10, 2025 15:51

TheBlueMatt added this to Weekly Goals Nov 13, 2025

TheBlueMatt self-assigned this Nov 13, 2025

valentinewallace reviewed Dec 3, 2025

View reviewed changes

TheBlueMatt force-pushed the 2025-10-pad-use-non-compact-bps branch from c7d1621 to 649ba34 Compare December 6, 2025 22:30

TheBlueMatt requested a review from valentinewallace December 6, 2025 22:30

valentinewallace reviewed Dec 9, 2025

View reviewed changes

lightning/src/onion_message/functional_tests.rs Outdated Show resolved Hide resolved

TheBlueMatt force-pushed the 2025-10-pad-use-non-compact-bps branch from 649ba34 to 8efb3e1 Compare December 9, 2025 21:18

ldk-reviews-bot requested a review from wpaulino December 9, 2025 21:20

wpaulino requested review from jkczyz and removed request for wpaulino December 10, 2025 18:49

jkczyz reviewed Dec 11, 2025

View reviewed changes

TheBlueMatt added 8 commits December 11, 2025 21:08

f rename to make it look less like a typo

9a83d1f

f sp

798981a

f sp

7428976

f sp + variable name

aded08d

f clearer comment

d8851b4

Add additional documentation on when to use NodeIdMessageRouter

4a048f0

Add a pending changelog note for breaking invreq blinded paths

f7620fe

TheBlueMatt force-pushed the 2025-10-pad-use-non-compact-bps branch from 8efb3e1 to f7620fe Compare December 11, 2025 21:08

		// We can only pad by a minimum of two bytes (we can only go from no-TLV to a type + length
		// byte). Thus, if there are any intermediate hops that need to be padded by exactly one

Default to padding blinded paths #4213

Are you sure you want to change the base?

Default to padding blinded paths #4213

Uh oh!

Conversation

TheBlueMatt commented Nov 10, 2025

Uh oh!

ldk-reviews-bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ldk-reviews-bot commented Nov 12, 2025

Uh oh!

ldk-reviews-bot commented Nov 15, 2025

Uh oh!

ldk-reviews-bot commented Nov 17, 2025

Uh oh!

ldk-reviews-bot commented Nov 19, 2025

Uh oh!

ldk-reviews-bot commented Nov 22, 2025

Uh oh!

ldk-reviews-bot commented Nov 24, 2025

Uh oh!

ldk-reviews-bot commented Nov 26, 2025

Uh oh!

ldk-reviews-bot commented Nov 29, 2025

Uh oh!

ldk-reviews-bot commented Dec 1, 2025

Uh oh!

ldk-reviews-bot commented Dec 3, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Dec 3, 2025

Uh oh!

ldk-reviews-bot commented Dec 8, 2025

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt commented Dec 9, 2025

Uh oh!

ldk-reviews-bot commented Dec 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Nov 10, 2025 •

edited

Loading

codecov bot commented Nov 10, 2025 •

edited

Loading